Multigrid: skip re-evaluation of unrefined nodes by pbrubeck · Pull Request #5288 · firedrakeproject/firedrake

pbrubeck · 2026-07-28T19:39:47Z

Stack

PR 2 of 4: #5337 (fix-dg-injection-child-count) -> this PR (adaptive-multigrid) -> #5270
(mesh-redistribution) -> #5287 (assign-submesh-restricted).

Description

AI-assisted (Claude Code)

An adaptive refinement leaves most of the mesh alone. Prolongation, restriction and injection
previously treated every fine node as if the whole mesh had changed: locate the coarse cell holding
it, pull it back to the coarse reference cell, and evaluate the coarse basis there. On a cell the
refinement did not touch, that reproduces the coarse dof it started from, to within roundoff — so
this PR skips the re-evaluation and copies those nodes' values directly.

Main changes

firedrake/cython/mgimpl.pyx: preserved_points finds, for a coarse/fine plex pair produced by
adaptive refinement, which fine points are exact copies of a coarse point. It compares closures
together with their orientations, not just their sizes, so a cell that refinement rebuilt
identically but reoriented is not mistaken for a preserved one.
firedrake/mg/utils.py: preserved_node_sf builds the PETSc.SF pairing the coarse and fine nodes
that correspond to those preserved points (via Section.distributeSection/createSectionSF,
trimmed to owned fine nodes only); transfer_node_subset gives the complementary Subset of fine
nodes the transfer kernels must still visit; prolong_preserved_nodes/restrict_preserved_nodes
do the copy/reduce for the nodes the SF accounts for.
firedrake/mg/interface.py: prolong/restrict run their op2.par_loop only over
transfer_node_subset's nodes, then call the preserved-node copy/reduce for the rest.

preserved_node_sf returns None when no node can be preserved. Its docstring names the four
reasons — mismatched node layouts, an extruded mesh, meshes that are not consecutive levels, and a
uniform refinement — and states that every rank reaches the same verdict, so that a caller may
branch on it collectively. Restriction adds rather than copies, at every stage: a coarse basis
function does not vanish on the neighbouring cells that refinement did split, so a preserved coarse
node also collects a contribution from the kernel.

Tests

tests/firedrake/multigrid/test_adaptive_multigrid.py. Four transfer tests (test_DG0, test_CG1,
test_restrict_CG1, test_restrict_DG0) fold into one parametrised test_transfers, which adds CG2
and CG3 and asserts that nodes really are copied rather than evaluated (_copied_nodes). It runs
across firedrake/netgen meshes, square/cube, at 1, 2, 3 and 4 processes.
test_grid_transfer.py is unchanged and still passes, which is what covers the uniform hierarchies
this PR must leave alone.

pbrubeck · 2026-08-05T08:22:12Z

+        # An adaptive refinement leaves most of the mesh alone, and the nodes
+        # it preserves are copied rather than evaluated.
+        node_subset = utils.transfer_node_subset(Vc, Vf)
+        op2.par_loop(kernel, node_subset, *kernel_args)
+        utils.prolong_preserved_nodes(coarse, fine)


This is the main part of the PR

pbrubeck · 2026-08-06T09:22:33Z

+        # cell's count. op2.Map cannot hold the -1 that marks a slot as empty,
+        # and the macro-cell kernel reading through this map integrates over
+        # every slot alike. Point all of an empty slot's nodes at a single one


I'm not sure how accurate this is.

It is certainly confusing.

macro-cell kernel

Huh?

empty slot's nodes

unclear

Context: DG injection integrates on a macro-cell formed by all fine children, which for an adaptive mesh are not uniform, so we need a ragged array with empty slots

I am asking about op2.Map not being able to hold a negative value

connorjward · 2026-08-06T10:33:59Z

-        op2.par_loop(kernel, fine.node_set, *kernel_args)
+        # An adaptive refinement leaves most of the mesh alone, and the nodes
+        # it preserves are copied rather than evaluated.
+        node_subset = utils.transfer_node_subset(Vc, Vf)


Suggested change

node_subset = utils.transfer_node_subset(Vc, Vf)

changed_node_subset = utils.transfer_node_subset(Vc, Vf)

clearer?

connorjward · 2026-08-06T10:38:03Z

+        # of the cell's own children's nodes: the slot then describes a
+        # degenerate cell sitting on the patch it belongs to, whose Jacobian
+        # determinant, and so whose contribution, vanishes.
+        if not valid.all():


I've just had to fix this in pyop3. Please remove this branch because it causes rank divergence. All ranks should just do it

What do you mean? Have you fixed this bug or a different one?

connorjward · 2026-08-06T10:42:54Z

+        # cell's count. op2.Map cannot hold the -1 that marks a slot as empty,
+        # and the macro-cell kernel reading through this map integrates over
+        # every slot alike. Point all of an empty slot's nodes at a single one


It is certainly confusing.

macro-cell kernel

Huh?

empty slot's nodes

unclear

connorjward · 2026-08-06T10:46:17Z

+    if Vc.extruded or Vf.extruded:
+        # The plex of an extruded mesh is the flat base one, whose points
+        # carry a whole column of nodes that the Sections cannot tell apart.
+        return None


Why can't we do extruded?

connorjward · 2026-08-06T10:47:45Z

+        kernel accumulated from the remaining fine nodes.
+
+    """
+    from firedrake.halo import _get_mtype


top level import

connorjward · 2026-08-06T10:48:40Z

+
+
+def restrict_preserved_nodes(fine_dual, coarse_dual):
+    """Add what the nodes an adaptive refinement preserved contribute to the coarse dual.


I don't understand why we're adding instead of copying

I think we should be able to copy because the operator is the identity on these nodes.

connorjward · 2026-08-06T10:50:28Z

+        CHKERR(DMPlexGetTransitiveClosure(fine_dm.dm, fine_point[child], PETSC_TRUE,
+                                          &fine_size, &fine_closure))
+        # A cell with one child that the transform nonetheless changed would
+        # have a closure of its own shape; leave it to the transfer kernel.


I don't understand this

I think the rule to detect preserved points is more subtle. A cell might be left unrefined but still changed by the transform?

Adaptive refinement only touches part of a mesh, so most fine cells are exact copies of a coarse one. Detect those copied cells via a PETSc SF over the points an adaptive refine_sbr transform preserves, and have prolong/restrict copy their nodes' values directly instead of running them through the transfer kernel, which is both cheaper and exact where evaluation would otherwise be approximate. Also pads the macro-cell coarse-to-fine node map's empty slots (where a coarse cell has fewer fine children than the busiest one in the hierarchy) with a degenerate cell of zero measure, so the map stays rectangular without the kernel double-counting real contributions.

- Drop the temporary FIAT install step now that fiat#267 is merged. - preserved_points computes nfine/ncoarse via num_owned_cells instead of taking nfine as an argument, asserting the passed-in array shape against it rather than trusting it as the source of truth.

Follow ASD-STE100: short sentences, one idea each, active voice, and the subject named up front rather than buried in a relative clause. The AGENTS.md rules this follows are in #5338. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…ltigrid

Name the four reasons no node can be preserved in one Notes section, and let each guard stand on its own. The collective agreement moves to where the verdict is reached, so _preserved_point_sf always returns an SF. Document why restriction accumulates instead of copying. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GRZG3iuNzgXQWGiuzrjKQo

pbrubeck added the base:main Run this PR using a main (dev) build label Jul 28, 2026

pbrubeck commented Jul 29, 2026

View reviewed changes

Comment thread firedrake/mg/interface.py

pbrubeck commented Jul 29, 2026

View reviewed changes

Comment thread firedrake/mg/utils.py Outdated

pbrubeck force-pushed the pbrubeck/adaptive-multigrid branch from fac303d to af6c7df Compare July 30, 2026 16:21

pbrubeck changed the base branch from pbrubeck/assign-submesh-restricted to pbrubeck/mg-redist July 30, 2026 16:21

This was referenced Jul 30, 2026

Redistributed Submesh and MeshHierarchy #5270

Open

Unify assign across Submesh, RestrictedFunctionSpace and RestrictedElement #5287

Open

pbrubeck changed the title ~~Multigrid: skip re-evaluation unrefined nodes~~ Multigrid: skip re-evaluation of unrefined nodes Jul 31, 2026

Base automatically changed from pbrubeck/mg-redist to main July 31, 2026 10:37

pbrubeck force-pushed the pbrubeck/adaptive-multigrid branch from af6c7df to 1a70084 Compare July 31, 2026 13:29

pbrubeck commented Jul 31, 2026

View reviewed changes

Comment thread .github/actions/install/action.yml Outdated

pbrubeck commented Aug 5, 2026

View reviewed changes

Comment thread tests/firedrake/multigrid/test_adaptive_multigrid.py

pbrubeck commented Aug 5, 2026

View reviewed changes

Comment thread firedrake/cython/mgimpl.pyx Outdated

pbrubeck commented Aug 5, 2026

View reviewed changes

Comment thread firedrake/cython/mgimpl.pyx Outdated

pbrubeck commented Aug 5, 2026

View reviewed changes

Comment thread firedrake/mg/utils.py

pbrubeck force-pushed the pbrubeck/adaptive-multigrid branch from 646da29 to 5625a7c Compare August 5, 2026 15:54

pbrubeck requested a review from connorjward August 6, 2026 09:20

pbrubeck commented Aug 6, 2026

View reviewed changes

connorjward added the ci:complex Run the test suite in complex mode label Aug 6, 2026

connorjward requested changes Aug 6, 2026

View reviewed changes

This was referenced Aug 8, 2026

Stop DG injection reading past a coarse cell's children #5337

Open

Agent tooling: sharper prose rules, a prose checker, and an fdk helper #5338

Closed

pbrubeck and others added 3 commits August 8, 2026 13:02

pbrubeck force-pushed the pbrubeck/adaptive-multigrid branch from 1c2b268 to 994901b Compare August 8, 2026 12:15

pbrubeck changed the base branch from main to pbrubeck/fix-dg-injection-child-count August 8, 2026 12:30

pbrubeck added 2 commits August 29, 2026 21:02

Merge pbrubeck/fix-dg-injection-child-count into pbrubeck/adaptive-mu…

a486445

…ltigrid

Merge pbrubeck/fix-dg-injection-child-count into pbrubeck/adaptive-mu…

da8b5ce

…ltigrid

pbrubeck added the LLM used An LLM was used in the production of this PR label Aug 29, 2026

	node_subset = utils.transfer_node_subset(Vc, Vf)
	changed_node_subset = utils.transfer_node_subset(Vc, Vf)



		def restrict_preserved_nodes(fine_dual, coarse_dual):
		"""Add what the nodes an adaptive refinement preserved contribute to the coarse dual.

Uh oh!

Conversation

pbrubeck commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Stack

Description

Main changes

Tests

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pbrubeck commented Jul 28, 2026 •

edited

Loading